Repository navigation
Pin coalescing under reports from many threads at once - #45
Merged
Merged
Conversation
report() runs on the host's threads, but no test called it from more than one. Eight threads now report one event 500 times each, and the first report plus the count close() sends add up to all 4000. Eight threads also open five windows each at the same moment, and every event is sent once and counted once, with nothing dropped below the queue and window limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Self-review rubric (head
Judgment calls, folded in here rather than posted as inline comments:
Summary: a test-only, green, in-scope Stage B PR. No path on the do-not-auto-merge list is touched. This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage B — unit-test expansion (coalescing under concurrent
report()calls).Summary
report()is called from the host's threads (thelockcomment inTraceClientsays so: "report() runs on the host's threads, flushes run on the sender thread"), but no test called it from more than one thread. Every existing coalescing test reports from the test thread alone.coalesce_reportsFromManyThreadsAtOnceAreEachCountedExactlyOnce: 8 threads report the same event 500 times each, all released by one latch. With the real one-minute window, exactly two events are sent: the first report, and the countclose()sends withcoalesced= 3999. This pins the documented counting formula (sum ofcoalescedwhere present, 1 where not) under contention.coalesce_distinctEventsFromManyThreadsAtOnceAreEachSentOnceAndCountedOnce: 8 threads open 5 windows each at the same moment and repeat each event 3 times. Every one of the 40 events is sent exactly once as a first report and once as a count of 2. Both totals are asserted to sit withinQUEUE_CAPACITYandMAX_COALESCED_EVENTS, so a drop would be a bug in the client, not the expected bound.runAtOncehelper starts the threads behind aCountDownLatchand joins them with a 10 s bound.main.Test plan
mvn -B verifylocally (Java 21):Tests run: 111, Failures: 0, Errors: 0, Skipped: 0, with both new tests present in the surefire reportbuild (8),build (17),build (21)green on the PR headSkipped issues
Every open issue is waiting on a maintainer decision, so none was picked for this cycle:
MAX_TAGSand theversionandcoalescedtags). These are contract changes.No tracking issue. The gap was found during Stage B triage.
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
🤖 Generated with Claude Code
drafted by Claude on behalf of Daniel Stephenson